Skip to content

fix: NetworkVariables of enum types breaking the inspector [MTT-6212]#2529

Merged
ShadauxCat merged 4 commits intodevelopfrom
fix/networkvariable_enums_in_inspector
May 2, 2023
Merged

fix: NetworkVariables of enum types breaking the inspector [MTT-6212]#2529
ShadauxCat merged 4 commits intodevelopfrom
fix/networkvariable_enums_in_inspector

Conversation

@ShadauxCat
Copy link
Copy Markdown
Collaborator

fixes #2444

Changelog

  • Fixed: Fixed the inspector throwing exceptions when attempting to render NetworkVariables of enum types.

Testing and Documentation

  • No tests have been added.
  • No documentation changes or additions were necessary.

@ShadauxCat ShadauxCat requested a review from a team as a code owner April 25, 2023 20:37
- Fixed issue where a client could throw an exception if abruptly disconnected from a network session with one or more spawned `NetworkObject`(s). (#2510)
- Fixed issue where invalid endpoint addresses were not being detected and returning false from NGO UnityTransport. (#2496)
- Fixed some errors that could occur if a connection is lost and the loss is detected when attempting to write to the socket. (#2495)
- Fixed the inspector throwing exceptions when attempting to render `NetworkVariable`s of enum types. (#2529)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we now render enum values in the inspector properly or do we just suppress/hide it? (didn't check the code yet :P but having it written in the changelog line would also be nice too, so I'm commenting this inline)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're rendered correctly. It's a fix to a regression, they're the way they were before the bug.

MethodInfo method;
if (isEquatable)
{
method = typeof(NetworkBehaviourEditor).GetMethod("RenderNetworkContainerValueTypeIEquatable", BindingFlags.Public | BindingFlags.Instance | BindingFlags.FlattenHierarchy | BindingFlags.NonPublic);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use nameof() here for the constant string "RenderNetworkContainerValueTypeIEquatable" value?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}
else
{
method = typeof(NetworkBehaviourEditor).GetMethod("RenderNetworkContainerValueType", BindingFlags.Public | BindingFlags.Instance | BindingFlags.FlattenHierarchy | BindingFlags.NonPublic);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ same for "RenderNetworkContainerValueType"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Copy Markdown
Contributor

@0xFA11 0xFA11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor suggestions but other than that, all good

- review feedback
- fixed an issue that could cause variables to render twice if the result of `NicifyVariableName()` is different from the actual variable name.
Copy link
Copy Markdown
Member

@NoelStephensUnity NoelStephensUnity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested under a couple of various conditions and it looks like that handles a couple of issues in one pass.
LGTM!

@ShadauxCat ShadauxCat enabled auto-merge (squash) May 2, 2023 17:27
@ShadauxCat ShadauxCat merged commit 1c58326 into develop May 2, 2023
@ShadauxCat ShadauxCat deleted the fix/networkvariable_enums_in_inspector branch May 2, 2023 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using an Enum in a NetworkVariable<T> breaks the inspector

3 participants